Conversation
awilfox
requested review from
anarchivist,
davezuckerman,
jason-raitz,
steve-sullivan and
yzhoubk
|
Example of what the login button looks like right now: 
|
davezuckerman
left a comment
davezuckerman
left a comment
There was a problem hiding this comment.
Looks good to me as far as the code goes. As for the questions.
I think the current button looks fine (maybe a bit boring) but I don't have a great artistic eye for that kind of stuff.
For the question about a user being logged in but not being an admin. It seems correct to raise the forbidden error. Even if it's a shared computer they should still be logging in as themself right?
yzhoubk
left a comment
yzhoubk
left a comment
There was a problem hiding this comment.
It looks good on my local and the workflow makes sense. Admin page looks solid, would be nice to get Jesse's thoughts too.
anarchivist
left a comment
anarchivist
left a comment
There was a problem hiding this comment.
r+.
fwiw, in avplayer, we styled the login button as a link - not sure that's the right case here, but we could argue that it's consistent.
* Display button on `UnauthorizedError` instead of redirecting to login. * Use `ForbiddenError` instead of redirecting to login when a user that is not an admin tries to access an admin page in the section for ref cards/stack passes. * Update specs.
awilfox
force-pushed
the
awilfox/AP-270-cas
branch
from
092c462 to
2dbb9ee
Compare
5 participants
Display button on
UnauthorizedErrorinstead of redirecting to login.Use
ForbiddenErrorinstead of redirecting to login when a user that is not an admin tries to access an admin page in the section for ref cards/stack passes.Update specs.
Potential TODOs: